Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql-server: Add behavior: auto_gc_behavior: enable. #8849

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

reltuk
Copy link
Contributor

@reltuk reltuk commented Feb 11, 2025

When Auto GC is enabled, the running sql-server will periodically collect a Dolt database that is growing in size. This behavior is currently experimental. Tuning the behavior around how often to collect is ongoing work.

When Auto GC is enabled, the running sql-server will periodically
collect a Dolt database that is growing in size. This behavior
is currently experimental. Tuning the behavior around how often to collect is
ongoing work.
@reltuk reltuk marked this pull request as ready for review February 11, 2025 03:55
@coffeegoddd
Copy link
Contributor

@reltuk DOLT

comparing_percentages
100.000000 to 100.000000
version result total
dc4b94d ok 5937457
version total_tests
dc4b94d 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@reltuk DOLT

comparing_percentages
100.000000 to 100.000000
version result total
9b53d20 ok 5937457
version total_tests
9b53d20 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@reltuk DOLT

comparing_percentages
100.000000 to 100.000000
version result total
62e5032 ok 5937457
version total_tests
62e5032 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@reltuk DOLT

comparing_percentages
100.000000 to 100.000000
version result total
f509d98 ok 5937457
version total_tests
f509d98 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@coffeegoddd DOLT

comparing_percentages
100.000000 to 100.000000
version result total
10dfcec ok 5937457
version total_tests
10dfcec 5937457
correctness_percentage
100.0

@reltuk reltuk requested a review from zachmu February 12, 2025 22:04
Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -113,7 +115,8 @@ func NewSqlEngine(
return nil, err
}

all := dbs[:]
all := make([]dsess.SqlDatabase, len(dbs))
copy(all, dbs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might add a note why making a copy here is important

if !ok {
if store != nil {
return store.ChunkJournal()
} else {
return nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this impossible? Store is set in both cases above

// delivers when no GC is currently running.
done chan struct{}
// It simplifies the logic and efficiency of the
// implementation a bit to have a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have a what?

type autoGCCommitHook struct {
c *AutoGCController
name string
// Always non-nil, if this channel delivers this channel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit of a run-on

@coffeegoddd
Copy link
Contributor

@reltuk DOLT

comparing_percentages
100.000000 to 100.000000
version result total
7c9c808 ok 5937457
version total_tests
7c9c808 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@reltuk DOLT

comparing_percentages
100.000000 to 100.000000
version result total
1e20b67 ok 5937457
version total_tests
1e20b67 5937457
correctness_percentage
100.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants